On delete account screen, add account number validation during input #5069
Conversation
IOS-278 Account deletion view enables OK button even if number is wrong
When entering the last 4 digits of the account number on the account deletion screen, the OK button lights up even if the number is incorrect. When pressing the OK button an error message is displayed, saying that the number is indeed incorrect. We could simplify this process by having the OK button function as validation instead and only enabling it if input is valid. |
rablador
force-pushed
the
account-deletion-view-enables-ok-button-even-if-number-is-ios-278
branch
from
4c1f369 to
9cc7a43
Compare
| let inputLengthIsValid = input.count == 4 | ||
| let inputMatchesAccountNumber = accountNumber.suffix(4) == input | ||
|
|
||
| return inputLengthIsValid && inputMatchesAccountNumber |
There was a problem hiding this comment.
Is inputLengthIsValid really needed?
accountNumber.suffix(4) == input seems to imply that input is indeed 4 characters long when it matches accountNumber.suffix(4)... ?
There was a problem hiding this comment.
The param in .suffix() specifies a max length, meaning the result could be less than that. In practice the account number - if present - should never be less than 4 digits, so for most (or perhaps all realistic) cases it should work as intended.
Theoretically though, accountNumber.suffix(4) could for some reason return something less than 4 digits, and if so the condition would be incorrect (eg. 3 digit account number matching 3 digit input). It's a safeguard, but perhaps it's not really necessary...
There was a problem hiding this comment.
In practice the account number - if present - should never be less than 4 digits
Yep this is what I assumed that'shy. But okay 👍 sorry for the bother!
| @@ -326,7 +335,7 @@ class AccountDeletionContentView: UIView { | |||
| } else { | |||
| activityIndicator.stopAnimating() | |||
| } | |||
| deleteButton.isEnabled = isDeleteButtonEnabled && isAccountNumberLengthSatisfied | |||
| deleteButton.isEnabled = isDeleteButtonEnabled && isInputValid | |||
There was a problem hiding this comment.
Should isDeleteButtonEnabled be updated too?
Currently it still says the button should be enabled upon failure:
There was a problem hiding this comment.
We want to disable the button when loading (to avoid interaction while trying to delete the account), otherwise enabled.
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @MrChocolatine)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MrChocolatine)
rablador
force-pushed
the
account-deletion-view-enables-ok-button-even-if-number-is-ios-278
branch
from
9cc7a43 to
39e1eab
Compare
pinkisemils
force-pushed
the
account-deletion-view-enables-ok-button-even-if-number-is-ios-278
branch
from
39e1eab to
1c9a3b0
Compare
pinkisemils
deleted the
account-deletion-view-enables-ok-button-even-if-number-is-ios-278
branch
When entering the last 4 digits of the account number on the account deletion screen, the OK button lights up even if the number is incorrect. When pressing the OK button an error message is displayed, saying that the number is indeed incorrect. The process has now been simplified by having the OK button itself function as validation instead and only enabling it if input is valid.
This change is